gh-132631: Fix json.tool when using --json-lines and a file as input #138961
Conversation
|
This is perhaps the most minimal fix, however, there's an alternative version that would be more invasive. Some observations:
|
mpkocher
changed the title
Lib/json/tool.py
Outdated
| for obj in objs: | ||
| json.dump(obj, outfile, **dump_args) | ||
| outfile.write('\n') | ||
| if infile is not sys.stdin: |
There was a problem hiding this comment.
This should be a finally I think. If there is an exception happening in json.dumps for instance, we would have it left open. So this should be moved after the except ValueError as e with some finally.
There was a problem hiding this comment.
I tried looking at the git history of this file and it's a bit tricky to understand why it's not using content managers. At one point it did have context manager approach, then it was migrated to manually calling of open.
If there's a friction point, it's the "in place" feature. I don't think the in place feature/requirement ever fundamentally worked for the json lines case.
Lib/json/tool.py
Outdated
| if options.json_lines: | ||
| objs = (json.loads(line) for line in infile) | ||
| else: | ||
| objs = (json.load(infile),) |
There was a problem hiding this comment.
This one can fail directl, so please be careful that we close the infile beforehand (that's the reaason why we have the try-finally in the first place).
There was a problem hiding this comment.
We're probably using a generator here to save memory. Otherwise we could've just used a list comprehension.
There was a problem hiding this comment.
My comment was about the else branch. It's not a generator, it's a regular tuple.
|
See PR #132632 for another attempt. |
hugovk
changed the title
|
The only test that is failing for me locally is the The "in place" functionality is a bit surprising and doesn't really have clear requirements around error handling. It's a bit disconcerting that in the "in place" mode you can overwrite/delete the original file if there's an error. |
4 participants
Uh oh!
There was an error while loading. Please reload this page.